-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
user notification for file uploads > 4GB in IE11 #27004
Conversation
@hurradieweltgehtunter, thanks for your PR! By analyzing the history of the files in this pull request, we identified @butonic, @PVince81 and @DeepDiver1975 to be potential reviewers. |
ieversion = 0; | ||
|
||
var ua = window.navigator.userAgent; | ||
var trident = ua.indexOf('Trident/'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix indent - we use tabs
@@ -28,6 +28,7 @@ OC.L10N.register( | |||
"Upload cancelled." : "Upload cancelled.", | |||
"Unable to upload {filename} as it is a directory or has 0 bytes" : "Unable to upload {filename} as it is a directory or has 0 bytes", | |||
"Total file size {size1} exceeds upload limit {size2}" : "Total file size {size1} exceeds upload limit {size2}", | |||
"Total file size {size1} exceeds your browser upload limit. Please use the owncloud desktop client to upload files bigger than 4GB." : "Total file size {size1} exceeds your browser upload limit. Please use the owncloud desktop client to upload files bigger than 4GB.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove - translations are handled via tranisfex
@@ -26,6 +26,7 @@ | |||
"Upload cancelled." : "Upload cancelled.", | |||
"Unable to upload {filename} as it is a directory or has 0 bytes" : "Unable to upload {filename} as it is a directory or has 0 bytes", | |||
"Total file size {size1} exceeds upload limit {size2}" : "Total file size {size1} exceeds upload limit {size2}", | |||
"Total file size {size1} exceeds your browser upload limit. Please use the owncloud desktop client to upload files bigger than 4GB." : "Total file size {size1} exceeds your browser upload limit. Please use the owncloud desktop client to upload files bigger than 4GB.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove - translations are handled via tranisfex
if (file.size > 4187593113) { | ||
data.textStatus = 'sizeexceedbrowserlimit'; | ||
data.errorThrown = t('files', | ||
'Total file size {size1} exceeds your browser upload limit. Please use the owncloud desktop client to upload files bigger than 4GB.', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
owncloud has to be passed in as argument due to whitelabling
@hurradieweltgehtunter Plz check if your size calculation matches the one from ownCloud. |
|
'size1': humanFileSize(file.size) | ||
}); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weird indents
@@ -336,6 +336,35 @@ OC.Upload = { | |||
data.errorThrown = errorMessage; | |||
} | |||
|
|||
// detect browser and version to handle IE11 upload file size limit | |||
// $.browser detects "mozilla" for IE11, which in this case we use window.navigator.userAgent | |||
ie = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use var
to declare these variables locally, else they'd be global
} | ||
|
||
// check browser and version | ||
if (ie && ieversion == 11) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we're doing this often, might be worth adding this as utility function to the OC.Util namespace which is currently in core/js/js.js (there is already a isIE()
function there)
@DeepDiver1975 Fix shouldn't be required for oC 10 (#26306) |
// detect browser and version to handle IE11 upload file size limit | ||
if (OC.Util.isIE11()) { | ||
// Check for the various File API support. | ||
if (window.File && window.FileReader && window.FileList && window.Blob) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really required or can we assume that IE11 has these ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ blob - http://caniuse.com/#search=Blob
please look up the rest @hurradieweltgehtunter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is completely unneeded because none of the checks below is using any of these APIs.
I'll remove it.
@@ -583,7 +601,7 @@ OC.Upload = { | |||
+ '</span><span class="mobile">' | |||
+ t('files', '...') | |||
+ '</span></em>'); | |||
$('#uploadprogressbar').tipsy({gravity:'n', fade:true, live:true}); | |||
$('#uploadprogressbar').tipsy({gravity:'n', fade:true, live:true}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hurradieweltgehtunter mind fixing this bad indent as well ?
I'll take care of the unit tests, it's a bit complicated. |
data.errorThrown = t('files', | ||
'Total file size {size1} exceeds your browser upload limit. Please use the {ownCloud} desktop client to upload files bigger than {size2}.', { | ||
'size1': humanFileSize(file.size), | ||
'ownCloud' : 'ownCloud', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs to use product name in case of white labeling
Added missing tests and fix theme name thingy |
@michaelstingl @davitol can you help test this in IE11 and non-IE11 envs with big files ? |
@hurradieweltgehtunter any please squash your 4 commits into one - thx |
63b3583
to
5e4f2b2
Compare
Rebased and squashed. Please review and test. |
👍 "Total file size 3.9 GB exceeds your browser upload limit. Please use the ownCloud desktop client to upload files bigger than 3.9 GB." message appears |
Travis, what's taking you so long ? Even Jenkins was faster... Anyway, Travis doesn't contain any JS related tests. Merging. |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Notify users with IE11 to use desktop client for uploading files bigger than 4GB.
Related Issue
https://github.com/owncloud/enterprise/issues/1779
Motivation and Context
IE11 can't handle file uploads bigger than 4GB
Screenshots (if appropriate):
Types of changes